Conversation
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
So the issue with I think having an agnostic tunnel implementation is worth having, but perhaps we should introduce a One thing to keep in mind is different implementations of the tunnel feature depends heavily on the framework itself, in Next.js it is implemented as simple re-write rules, so we almost don't have any runtime logic for it on the serve-side. cc @Lms24 |
This is not the case because of tree-shaking. We have the If a helper like this ends up in the Node SDK it won't be usable in non-Node based SDKs. |
| // This prevents SSRF attacks where attackers send crafted envelopes | ||
| // with malicious DSNs pointing to arbitrary hosts | ||
| const isAllowed = allowedDsnComponents.some( | ||
| allowed => allowed.host === dsnComponents.host && allowed.projectId === dsnComponents.projectId, |
There was a problem hiding this comment.
I'm not sure it's enough to just check the host matches. We should have a list of allowed DSNs and only forward when they match.
There was a problem hiding this comment.
Yeah the allowedDsnComponents are passed from the outside and they're exactly that - a list of allowed DSNs.
There was a problem hiding this comment.
maybe instead of turning them in components we should pass them as string arrays and do a plain string comparison?
3 participants
Overview
Right now only the Next.js SDK has a built-in tunnel handler on the backend side, but this PR aims to extract the tunnel handling logic in plain JavaScript in
@sentry/core, and provide framework-specific adapters.Framework-specific adapters checklist
Questions for the team
@sentry/corea good home for the framework-agnostic tunnel handler function?TODOs